Skip to content

feat(security): Add audience and issuer validation to OIDC JWT authentication#26781

Open
kimsehwan96 wants to merge 1 commit intoopen-metadata:mainfrom
kimsehwan96:fix/oidc-aud-iss-validation
Open

feat(security): Add audience and issuer validation to OIDC JWT authentication#26781
kimsehwan96 wants to merge 1 commit intoopen-metadata:mainfrom
kimsehwan96:fix/oidc-aud-iss-validation

Conversation

@kimsehwan96
Copy link

Describe your changes:

JWT token validation in OpenMetadata only checks signature and expiry, but does not validate aud (audience) or iss (issuer) claims. This means a token issued by the same OIDC provider (e.g., Authentik) but for a different client ID can pass authentication — essentially a cross-client token reuse vulnerability.

What I changed:

  • Added issuer/audience validation in JwtFilter.validateJwtAndGetClaims()
  • Added issuer/audience validation in AuthenticationCodeFlowHandler for the OIDC code flow
  • Internal tokens (bots, PATs) are identified by matching the internal JWT issuer and bypass OIDC-specific validation
  • Validation is skipped when clientId/authority is not configured for backward compatibility
  • Added @Getter to JWTTokenGenerator.issuer so JwtFilter can identify internal tokens

Type of change:

  • Bug fix
  • Improvement
  • New feature
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation

Checklist:

  • I have read the CONTRIBUTING document.
  • My PR title is Fixes <issue-number>: <short explanation>
  • I have commented on my code, particularly in hard-to-understand areas.
  • For JSON Schema changes: I updated the migration scripts or explained why it is not needed.

Copilot AI review requested due to automatic review settings March 26, 2026 01:12
@github-actions
Copy link
Contributor

Hi there 👋 Thanks for your contribution!

The OpenMetadata team will review the PR shortly! Once it has been labeled as safe to test, the CI workflows
will start executing and we'll be able to make sure everything is working as expected.

Let us know if you need any help!

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds issuer (iss) and audience (aud) validation to OpenMetadata’s JWT authentication flow to prevent cross-client token reuse when using OIDC, while keeping internal OpenMetadata-issued tokens working.

Changes:

  • Add iss/aud checks in JwtFilter.validateJwtAndGetClaims() when authority/clientId are configured, with a bypass path for internal tokens.
  • Validate iss/aud for the OIDC authorization code flow ID token handling.
  • Expand JwtFilterTest coverage for issuer/audience validation scenarios; expose JWTTokenGenerator.issuer via Lombok @Getter.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.

File Description
openmetadata-service/src/main/java/org/openmetadata/service/security/JwtFilter.java Adds configured iss/aud validation and internal-token bypass logic during JWT claim extraction.
openmetadata-service/src/main/java/org/openmetadata/service/security/AuthenticationCodeFlowHandler.java Adds iss/aud validation for the ID token received in the OIDC code flow.
openmetadata-service/src/main/java/org/openmetadata/service/security/jwt/JWTTokenGenerator.java Exposes the internal JWT issuer via getter for internal token identification.
openmetadata-service/src/test/java/org/openmetadata/service/security/JwtFilterTest.java Adds unit tests covering audience/issuer validation and bypass behavior.

Comment on lines +316 to +317
boolean isInternalToken =
internalJwtIssuer != null && internalJwtIssuer.equals(jwt.getIssuer());
Copy link

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

isInternalToken is determined solely by comparing the token iss claim to the configured internal JWT issuer. If an operator configures the internal jwtissuer to match the external OIDC issuer (or they coincide), then external OIDC tokens would be treated as “internal” and bypass iss/aud validation, reintroducing the cross-client token reuse risk this PR is addressing. Consider identifying internal tokens using a stronger signal tied to the signing key (e.g., token kid matching the locally configured JWKS key id) and/or the presence of OpenMetadata-only claims (like tokenType), rather than issuer string equality alone.

Suggested change
boolean isInternalToken =
internalJwtIssuer != null && internalJwtIssuer.equals(jwt.getIssuer());
Claim tokenTypeClaim = jwt.getClaim(TOKEN_TYPE);
boolean hasInternalTokenTypeClaim = tokenTypeClaim != null && !tokenTypeClaim.isNull();
boolean isInternalToken =
internalJwtIssuer != null
&& internalJwtIssuer.equals(jwt.getIssuer())
&& hasInternalTokenTypeClaim;

Copilot uses AI. Check for mistakes.
@harshach harshach added the safe to test Add this label to run secure Github workflows on PRs label Mar 26, 2026
@harshach
Copy link
Collaborator

Thanks @kimsehwan96 for the PR. please check the co-pilot reivew comments if they are relevant to address

@github-actions
Copy link
Contributor

The Java checkstyle failed.

Please run mvn spotless:apply in the root of your repository and commit the changes to this PR.
You can also use pre-commit to automate the Java code formatting.

You can install the pre-commit hooks with make install_test precommit_install.

@kimsehwan96
Copy link
Author

I added more changes.

  • Issuer is now resolved from the OIDC discovery document (/.well-known/openid-configuration) instead of using authority directly.
  • Issuer comparison is normalized (trailing slash removal, case insensitive).
  • Error messages no longer expose expected issuer/clientId values. Details are logged server-side via LOG.warn
  • Added null guards for provider metadata and audience claims.

I've tested this with Authentik (custom-oidc).
Discovery-based issuer resolution is implemented for other providers (Azure AD, Keycloak, etc.) but I don't have those environments.

If anyone can verify with Azure AD or similar, that would be appreciated.

@github-actions
Copy link
Contributor

github-actions bot commented Mar 26, 2026

🟡 Playwright Results — all passed (18 flaky)

✅ 3413 passed · ❌ 0 failed · 🟡 18 flaky · ⏭️ 209 skipped

Shard Passed Failed Flaky Skipped
🟡 Shard 1 453 0 2 2
🟡 Shard 2 603 0 2 32
🟡 Shard 3 612 0 3 28
🟡 Shard 4 599 0 4 47
✅ Shard 5 587 0 0 67
🟡 Shard 6 559 0 7 33
🟡 18 flaky test(s) (passed on retry)
  • Features/DataAssetRulesDisabled.spec.ts › Verify the SearchIndex Service entity item action after rules disabled (shard 1, 1 retry)
  • Pages/UserCreationWithPersona.spec.ts › Create user with persona and verify on profile (shard 1, 1 retry)
  • Features/BulkEditEntity.spec.ts › Glossary (shard 2, 1 retry)
  • Features/BulkImport.spec.ts › Database (shard 2, 1 retry)
  • Features/Permissions/GlossaryPermissions.spec.ts › Team-based permissions work correctly (shard 3, 1 retry)
  • Features/TestSuitePipelineRedeploy.spec.ts › Re-deploy all test-suite ingestion pipelines (shard 3, 1 retry)
  • Flow/ExploreDiscovery.spec.ts › Should display deleted assets when showDeleted is checked and deleted is not present in queryFilter (shard 3, 1 retry)
  • Pages/Customproperties-part2.spec.ts › entityReferenceList shows item count, scrollable list, no expand toggle (shard 4, 1 retry)
  • Pages/Entity.spec.ts › Delete Directory (shard 4, 1 retry)
  • Pages/Entity.spec.ts › UpVote & DownVote entity (shard 4, 1 retry)
  • Pages/Entity.spec.ts › Tag Add, Update and Remove (shard 4, 1 retry)
  • Pages/Glossary.spec.ts › Async Delete - multiple deletes with mixed results (shard 6, 1 retry)
  • Pages/HyperlinkCustomProperty.spec.ts › should show No Data placeholder when hyperlink has no value (shard 6, 1 retry)
  • Pages/Lineage.spec.ts › Lineage creation from SearchIndex entity (shard 6, 1 retry)
  • Pages/ODCSImportExport.spec.ts › Multi-object ODCS contract - object selector shows all schema objects (shard 6, 1 retry)
  • Pages/ServiceEntity.spec.ts › Set & Update table-cp, hyperlink-cp, string, integer, markdown, number, duration, email, enum, sqlQuery, timestamp, entityReference, entityReferenceList, timeInterval, time-cp, date-cp, dateTime-cp Custom Property (shard 6, 1 retry)
  • Pages/Users.spec.ts › Permissions for table details page for Data Consumer (shard 6, 1 retry)
  • VersionPages/EntityVersionPages.spec.ts › Directory (shard 6, 1 retry)

📦 Download artifacts

How to debug locally
# Download playwright-test-results-<shard> artifact and unzip
npx playwright show-trace path/to/trace.zip    # view trace

Copilot AI review requested due to automatic review settings March 26, 2026 04:44
@kimsehwan96 kimsehwan96 force-pushed the fix/oidc-aud-iss-validation branch from 2edf2f4 to 3e88eea Compare March 26, 2026 04:44
@github-actions
Copy link
Contributor

The Java checkstyle failed.

Please run mvn spotless:apply in the root of your repository and commit the changes to this PR.
You can also use pre-commit to automate the Java code formatting.

You can install the pre-commit hooks with make install_test precommit_install.

@kimsehwan96 kimsehwan96 force-pushed the fix/oidc-aud-iss-validation branch from 3e88eea to d894ac0 Compare March 26, 2026 04:47
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.

Comment on lines +155 to +159
String discoveryUrl = authority;
if (!discoveryUrl.endsWith("/")) {
discoveryUrl += "/";
}
discoveryUrl += ".well-known/openid-configuration";
Copy link

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

resolveOidcIssuer performs an HTTP call to the IdP discovery endpoint during JwtFilter construction. If the IdP is slow/unreachable, this can delay startup (15s default timeout) and adds an external dependency to initialize the auth filter. Consider resolving/caching the issuer during configuration validation/save, or making discovery resolution explicitly optional/non-blocking with a shorter timeout.

Copilot uses AI. Check for mistakes.
…tication

OpenMetadata's JWT token validation only checked signature and expiry,
allowing cross-client token reuse within the same OIDC provider instance.

Changes:
- Add iss/aud claim validation in JwtFilter.validateJwtAndGetClaims()
- Add iss/aud validation in AuthenticationCodeFlowHandler for OIDC code flow
- Resolve actual OIDC issuer from discovery document instead of using
  authority URL directly (fixes Azure AD v2.0 and similar providers)
- Normalize issuer comparison (trailing slash, case insensitive)
- Internal tokens (bots, PATs) identified by matching internal JWT issuer
  bypass OIDC-specific validation
- Error messages return generic info to clients, details logged server-side
- Null guards for provider metadata and audience claims
- Validation skipped when clientId/authority not configured (backward compat)
- Expose JWTTokenGenerator.issuer via @Getter for internal token detection
@kimsehwan96 kimsehwan96 force-pushed the fix/oidc-aud-iss-validation branch from d894ac0 to 29a5ee8 Compare March 26, 2026 04:56
@gitar-bot
Copy link

gitar-bot bot commented Mar 26, 2026

Code Review ✅ Approved 5 resolved / 5 findings

Adds audience and issuer validation to OIDC JWT authentication, resolving security issues including proper issuer validation using authority URLs, signature origin verification for internal tokens, sensitive error message redaction, locale-independent URL normalization, and RFC 3986-compliant path handling.

✅ 5 resolved
Bug: Issuer validation uses authority URL, not actual OIDC issuer

📄 openmetadata-service/src/main/java/org/openmetadata/service/security/JwtFilter.java:142 📄 openmetadata-service/src/main/java/org/openmetadata/service/security/JwtFilter.java:320-328
In JwtFilter.java:142, expectedIssuer is set from authenticationConfiguration.getAuthority(), which is the base authority URL (e.g., https://login.microsoftonline.com/{tenant}). However, many OIDC providers return a different value in the iss claim — for example, Azure AD v2.0 tokens have iss: https://login.microsoftonline.com/{tenant}/v2.0. Since validation at line 322 uses exact string comparison, this will cause all legitimate external tokens to be rejected for such providers.

The OIDC discovery endpoint already resolves the actual issuer (see CustomOidcValidator.java:174 which reads discoveryDoc.path("issuer").asText()), but that resolved value is not persisted in AuthenticationConfiguration or made available to JwtFilter.

This is a breaking change for Azure AD (and potentially other providers) when authority is configured, which is likely common in production deployments.

Security: Internal token bypass only checks issuer, not signature origin

📄 openmetadata-service/src/main/java/org/openmetadata/service/security/JwtFilter.java:316-317
In JwtFilter.java:316-317, any token whose iss claim matches internalJwtIssuer bypasses audience and issuer validation entirely. Since the signature is validated earlier (line 307-313) against the JWK provider which includes the internal public key, this is safe only if external OIDC providers cannot produce tokens with the same iss value. However, if the internal issuer string (e.g., open-metadata.org) is predictable/known, and the JWK provider also serves the external provider's keys, an external token could potentially be crafted to match.

In practice, this is mitigated because the signature must validate against a known key, but the intent would be clearer and more robust if the bypass also checked the kid (key ID) or verified that the signing key belongs to the internal key pair specifically.

Security: Error messages leak expected issuer and client ID values

📄 openmetadata-service/src/main/java/org/openmetadata/service/security/JwtFilter.java:324-327 📄 openmetadata-service/src/main/java/org/openmetadata/service/security/JwtFilter.java:335-338 📄 openmetadata-service/src/main/java/org/openmetadata/service/security/AuthenticationCodeFlowHandler.java:1197-1200 📄 openmetadata-service/src/main/java/org/openmetadata/service/security/AuthenticationCodeFlowHandler.java:1206-1209
The error messages at lines 324-327 and 335-338 in JwtFilter.java (and similarly in AuthenticationCodeFlowHandler.java:1197-1200, 1206-1209) include the expected issuer and client ID values. While useful for debugging, exposing these configuration details in error responses to unauthenticated callers could help an attacker craft more targeted tokens. Consider logging the full details server-side and returning a generic error message to the client.

Edge Case: normalizeIssuer uses locale-sensitive toLowerCase()

📄 openmetadata-service/src/main/java/org/openmetadata/service/security/JwtFilter.java:192
String.toLowerCase() without a Locale argument is locale-sensitive. In a JVM running with Turkish locale, 'I' lowercases to 'ı' (dotless i) rather than 'i', which could cause issuer URLs containing uppercase I to fail comparison unexpectedly. Since this is used for security-critical issuer comparison, it should use Locale.ROOT to guarantee consistent ASCII lowercasing.

Security: normalizeIssuer lowercases URL path, violating RFC 3986

📄 openmetadata-service/src/main/java/org/openmetadata/service/security/JwtFilter.java:185-193
normalizeIssuer() applies toLowerCase(Locale.ROOT) to the entire issuer URL, including the path component. Per RFC 3986 §6.2.2.1, only the scheme and host are case-insensitive; the path is case-sensitive. The OIDC Discovery spec (§4.3) also requires simple string comparison for the issuer. This means two distinct issuers that differ only in path case (e.g., https://example.com/Realm vs https://example.com/realm) would incorrectly be treated as equal. In practice this is unlikely to be exploitable since OIDC providers use stable, lowercase paths, but it's a spec deviation worth noting.

Options

Auto-apply is off → Gitar will not commit updates to this branch.
Display: compact → Showing less information.

Comment with these commands to change:

Auto-apply Compact
gitar auto-apply:on         
gitar display:verbose         

Was this helpful? React with 👍 / 👎 | Gitar

@sonarqubecloud
Copy link

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

safe to test Add this label to run secure Github workflows on PRs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants